wasmtime: don't propagate binaries for C API & library#438391
wasmtime: don't propagate binaries for C API & library#438391pyrox0 merged 3 commits intoNixOS:masterfrom
Conversation
|
Since I've contributed the C API and done a bit of continuous troubleshooting/optimizing on it, I'd be glad to be added as a maintainer if the current maintainers are happy with that. |
Absolutely! Feel free to add yourself in this PR! |
ereslibre
left a comment
There was a problem hiding this comment.
Thanks for the contribution, it's great to have smaller closures!
|
ereslibre
left a comment
There was a problem hiding this comment.
Approved automatically following the successful run of nixpkgs-review.
|
@NixOS/nixpkgs-merge-bot merge |
|
@ereslibre merge not permitted (#305350): |
|
Oh, I just realized that if we turned building the static library into an opt-in, we could get rid of another 117M: The reason I'm obsessing about this a bit is that #394870 would ask people to pull --- a/pkgs/by-name/wa/wasmtime/package.nix
+++ b/pkgs/by-name/wa/wasmtime/package.nix
@@ -6,6 +6,7 @@
cmake,
versionCheckHook,
nix-update-script,
+ enableStatic ? stdenv.hostPlatform.isStatic,
}:
rustPlatform.buildRustPackage (finalAttrs: {
pname = "wasmtime";
@@ -63,6 +64,9 @@ rustPlatform.buildRustPackage (finalAttrs: {
# https://github.com/rust-lang/cargo/issues/9661
cp -r target/${cargoShortTarget}/release/build/wasmtime-c-api-impl-*/out/include $dev/include
''
+ + lib.optionalString (!enableStatic) ''
+ rm $dev/lib/*.a
+ ''
+ lib.optionalString stdenv.hostPlatform.isDarwin ''
install_name_tool -id \
$dev/lib/libwasmtime.dylib \There's some prior art for removing the static libraries like that in tree-sitter itself: nixpkgs/pkgs/development/tools/parsing/tree-sitter/default.nix Lines 210 to 214 in da5efcc |
|
@nekowinston thanks for the context. I think it would make sense to change that as another PR, and I'd be fine approving that. I didn't have the real need to optimize that much, but I think it makes sense if it helps for the |
|
Alright, I'll open a follow-up PR for that 🙂 |
|
Hey @nekowinston! I hit an issue that was caused by this PR, and wanted to check whether it's intentional or just a bug: nix-shell --pure -I nixpkgs=$PWD -p wasmtime --run "wasmtime --version"As of 3f405f0 and later, that command fails and prints If it was intentional to remove |
Reduces the closure size if just the library/C API is used.
According to
nix-tree, running on./result-dev:before:
NAR Size: 142.75 MiB | Closure Size: 233.31 MiB | Added Size: 233.31 MiB
after:
NAR Size: 142.92 MiB | Closure Size: 183.94 MiB | Added Size: 183.94 MiB
I've also noticed that there's a preexisting function (moveToOutput) doing what's done manually; it just doesn't seem to be documented anywhere.
Things done
passthru.tests.nixpkgs-reviewon this PR. See nixpkgs-review usage../result/bin/.Add a 👍 reaction to pull requests you find important.